Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

PVI update to match Investopedia definition and fix for issue 625 #779

Merged
merged 2 commits into from
Jun 11, 2024
Merged

PVI update to match Investopedia definition and fix for issue 625 #779

merged 2 commits into from
Jun 11, 2024

Conversation

Rossco8
Copy link

@Rossco8 Rossco8 commented Mar 17, 2024 •
edited by twopirllc
Loading

There are 2 PVI indicators on TradingView.

One calculates PVI using

pvi := na(pvi[1]) ? initial : (change(volume) > 0 ? pvi[1] * close / close[1] : pvi[1])

The other uses

nRes = iff(volume > volume[1], nz(nRes[1], 0) + xROC, nz(nRes[1], 0))

Both of them are different, and neither of them follow the logic outlined on most websites, e.g.

https://www.investopedia.com/terms/p/pvi.asp
https://www.kotaksecurities.com/share-market/what-is-positive-volume-index/
https://www.barchart.com/education/technical-indicators/positive_volume_index
https://www.angelone.in/knowledge-center/share-market/pvi

Which all show the formula as

Yesterday’s PVI + [[(Close – Yesterday’s Close) / Yesterday’s Close] * Yesterday’s PVI

There are other definitions of the formula available online, the authors of the TV indicators may have used

The source code referenced in issue #625 at the voice32 repo uses

pvi = prev_pvi + (row[close_col] - prev_close / prev_close * prev_pvi)

Which is almost the Investopedia version, if it had (row[close_col] - prev_close) in parentheses

So this PR includes the following - NOTE: This is a breaking change as the new PVI() returns a DataFrame rather than a Series

  1. Loop through arrays rather than DF rows for performance improvements
  2. initial value set to 100 rather than 1000 which seems to be the norm amongst the other implementations
  3. Default length parameter set to 255 to match the other implementation
  4. added mamode with a default of EMA
  5. returns a DataFrame rather than a Series

Performance of the new PVI implementation is roughly twice as fast than previous version

3275 function calls (3214 primitive calls) in 0.012 seconds

   Ordered by: internal time

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
        1    0.001    0.001    0.001    0.001 datetimes.py:545(_box_func)
        1    0.001    0.001    0.011    0.011 pvi.py:77(pvi_new_vect)
        1    0.001    0.001    0.001    0.001 rolling.py:601(calc)
        1    0.001    0.001    0.001    0.001 sre_parse.py:494(_parse)
        2    0.000    0.000    0.002    0.001 indexing.py:1176(__getitem__)
  565/564    0.000    0.000    0.000    0.000 {built-in method builtins.isinstance}
        6    0.000    0.000    0.002    0.000 frame.py:3971(_ixs)
        4    0.000    0.000    0.001    0.000 base.py:475(__new__)
        3    0.000    0.000    0.001    0.000 base.py:6956(insert)
        5    0.000    0.000    0.001    0.000 frame.py:4050(__getitem__)
        3    0.000    0.000    0.003    0.001 managers.py:1347(insert)

Performance of the original PVI implementation

5088 function calls (4995 primitive calls) in 0.020 seconds

   Ordered by: internal time

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
        2    0.003    0.001    0.003    0.002 algorithms.py:1339(diff)
        1    0.001    0.001    0.001    0.001 base.py:411(_outer_indexer)
        3    0.001    0.000    0.001    0.000 datetimelike.py:388(_get_getitem_freq)
        1    0.000    0.000    0.020    0.020 <string>:1(<module>)
       12    0.000    0.000    0.002    0.000 series.py:389(__init__)
  996/979    0.000    0.000    0.001    0.000 {built-in method builtins.isinstance}
        2    0.000    0.000    0.002    0.001 frame.py:4050(__getitem__)
        1    0.000    0.000    0.018    0.018 pvi.py:169(pvi)
        2    0.000    0.000    0.004    0.002 generic.py:10612(_where)
       26    0.000    0.000    0.000    0.000 generic.py:6233(__finalize__)
        1    0.000    0.000    0.020    0.020 {built-in method builtins.exec}
       16    0.000    0.000    0.002    0.000 managers.py:317(apply)
        2    0.000    0.000    0.004    0.002 series.py:3026(diff)
  352/337    0.000    0.000    0.000    0.000 {built-in method builtins.getattr}

@Rossco8
Copy link
Author

Rossco8 commented Mar 17, 2024

Hi @twopirllc I can not see why the automated tests are failing for pvi. Any suggestions?

@twopirllc
Copy link
Owner

@Rossco8

On first glance, it is odd that the test_pvi and test_ext_pvi are not passing since they are returning a two column dataframe. 🤷🏼‍♂️ But I will dig into it.

I am currently working on td_seq PR #774 and testing some misc numpy/numba code.

twopirllc added a commit that referenced this pull request Jun 11, 2024
@twopirllc
#779 #625 #775 BUG ENH pvi update with numba MAINT use to_numpy()
930fa15
twopirllc added a commit that referenced this pull request Jun 11, 2024
@twopirllc
BUG MAINT TST #779 pvi update
fa7082b
@twopirllc twopirllc merged commit 930d7df into twopirllc:development Jun 11, 2024
1 check failed
@Rossco8 Rossco8 deleted the pvi.fix.issue.625 branch June 11, 2024 21:14
@twopirllc
Copy link
Owner

Hey @Rossco8

Apologies for the delay. Finally got a moment to get to this 😅.

I converted it to numba using a simplified version of:
Yesterday’s PVI + [[(Close – Yesterday’s Close) / Yesterday’s Close] * Yesterday’s PVI that you implemented which is also the version Everget uses on TV. I also included an overlap argument similar to Sierra Chart.

Since we are running different machines, I am curious what the numba performance is like compared to your submitted version.

KJ

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Rossco8 @twopirllc